[feat/#8] 4주차 필수 과제 구현 #9
Conversation
회원가입 관련 userService 추가 및 retrofit okhttp provide 구현
WavveActionTextField에서도 키보드 옵션 인자 추가
hobby와 SnackBarText 추가
kangyein9892
requested review from
jm991014,
boiledEgg-s,
jihyunniiii and
serioushyeon
| import org.json.JSONObject | ||
| import org.json.JSONException | ||
|
|
||
| inline fun <T> execute(block: () -> T): Result<T> = runCatching { |
| import kotlinx.serialization.Serializable | ||
| import org.sopt.and.domain.model.MyHobbyResponse | ||
|
|
||
| @Serializable |
| dependencies { | ||
| implementation(project(":data")) | ||
| implementation(project(":domain")) | ||
| implementation(project(":presentation")) |
| override fun onCreate() { | ||
| super.onCreate() | ||
| PreferenceUtil.init(this) | ||
| } |
There was a problem hiding this comment.
궁금한 점!! 지현이 코드에도 이렇게 구현되어 있는데, 초기화 작업이 따로 없는 것 같은데 onCreate() 함수를 작성한 이유가 따로 있나요
There was a problem hiding this comment.
아뇨...! 원래 PreferenceUtil.init 때문에 썼던 거였습니다...!!
| suspend fun signIn(request: SignInRequestDto): Result<SignInResponse> = execute { | ||
| authService.signIn(request).result.toDomainModel() | ||
| } |
| when(networkState){ | ||
| is NetworkState.Loading -> { | ||
| // TODO 로딩 추가 | ||
| } | ||
| is NetworkState.Error -> { | ||
| viewModel.handleSignUpIntentError(networkState.title + networkState.msg) | ||
| } | ||
| is NetworkState.Success -> { | ||
| viewModel.handleSignUpIntentSuccess() | ||
| } |
There was a problem hiding this comment.
Qualifier가 필요없어 보이는데 사용하신 이유가 있을까요??
There was a problem hiding this comment.
엇 필요하다고 생각했는데 생각해보니 필요없을 것 같네요! 수정하겠습니다!
| val username: String | ||
| ) | ||
|
|
||
| internal fun User.toRequestBody(): SignUpRequestDto { |
There was a problem hiding this comment.
User의 확장 함수가 dto 파일에 들어있으면 파일의 책임이 너무 모호해진다고 생각합니다!
저는 mapper라는 패키지로 관리하는 것을 선호합니다!
| fun toDomainModel(): MyHobbyResponse { | ||
| return MyHobbyResponse( | ||
| hobby = hobby | ||
| ) | ||
| } |
There was a problem hiding this comment.
dto -> model 변환도 매퍼 패키지에서 관리하면 책임 분리가 명확해진다고 생각합니다!
There was a problem hiding this comment.
presentation에서 네트워크 통신에서 발생하는 에러를 처리해주고 관리해주는 클래스라고 생각하시면 될 것 같아요!
| import org.json.JSONObject | ||
| import org.json.JSONException | ||
|
|
||
| inline fun <T> execute(block: () -> T): Result<T> = runCatching { |
|
|
||
| var id: String | ||
| get() = userSharedPreference.getString(ID, "").toString() | ||
| set(value) = userSharedPreference.edit().putString(ID, value).apply() |
There was a problem hiding this comment.
edit로 데이터를 변경할 준비를 하고 그 뒤에 putString으로 변경사항을 정의해주고, apply로 이 정의된 변경사항을 실제로 적용시켜주는 개념으로 이해하고 있습니다...!
| @InstallIn(SingletonComponent::class) | ||
| internal interface RepositoryModule { | ||
|
|
||
| @Binds |
| @@ -0,0 +1,5 @@ | |||
| package org.sopt.and.domain.model | |||
|
|
|||
| data class MyHobbyResponse( | |||
There was a problem hiding this comment.
domain에 있는 data class에 Response라는 네이밍이 사용되면서 너무 data 레이어스러운 네이밍이 된 것 같습니다! (개인적으로 ㅋㅋ)
한 번 어떻게 네이밍하는 게 좋을지 고민해보면 좋을 것 같습니다.
| val updatedException = when(exception){ | ||
| is HttpCodeError -> { | ||
| when(exception.title) { | ||
| "400" -> HttpCodeError( |
| init { | ||
| getMyHobby() | ||
| } |
There was a problem hiding this comment.
init 블록은 어떤 역할을 할까요? 그리고 어떠한 로직들을 담는 게 좋을까요?
There was a problem hiding this comment.
클래스 초기화시 실행되는 코드입니다. 필수로 실행되어야하는 로직들을 담으면 좋을 것 같습니다...!!
There was a problem hiding this comment.
init 블록에서 무거운 작업(서버 통신, 데이터베이스 쿼리 등과 같은,,)을 진행하는 것이 좋지 않고, 가벼운 초기화 로직만 들어가는 게 좋습니다.
init 블록에서 가벼운 초기화 로직만 작성해야 하는 이유는 여러 가지가 있는데요,, 일단 첫번째로 init 블럭은 사실상 생성자의 일부로 볼 수 있어서 비동기로 처리되어야 하는 로직을 작성하는 게 좋지 않습니다. 그리고 init 블럭에서 함수를 호출하면 그 시점이 앱의 다른 생명주기 이벤트와 충돌할 수 있어 문제가 됩니다. (예를 들어,, 뷰모델이 액티비티보다 먼저 초기화 되었는데 그 부분에서 UI를 업데이트 시키는 경우 -> 이로 인한 NPE) 또, 뷰모델에서 의존성 주입을 하는 경우에는 의존성이 주입되기 전에 함수가 호출될 수 있어요!
추가로 저도 리뷰 달다가 본 글인데 (종명 오빠 땡큐 ㅎ.ㅎ) 이 글 읽어보셔도 좋을 것 같네요 ~
| @Inject | ||
| lateinit var networkDelegate: NetworkDelegate |
There was a problem hiding this comment.
생성자에 넣지 않고 따로 inject 시켜준 이유가 있나요?
There was a problem hiding this comment.
엇 아뇨 따로 이유는 없고 사실 생성자로 다 수정하려고 했는데 수정을 다 못한 것 같네요...!
Related issue 🛠
Work Description ✏️
Screenshot 📸
회원가입
default.mp4
default.mp4
로그인
default.mp4
default.mp4
마이페이지
Uncompleted Tasks 😅
To Reviewers 📢
미리 사과의 말씀을 드리면... 바뀐 것도 많고 보기도 힘드실 수도 있습니다...^^
감사드려요...
혼자서 처음부터 끝까지 hilt랑 멀티모듈 적용하고, 클린아키텍쳐 적용하는게 처음이라 잘한 건지 모르겠네요...! 잘부탁드려요~~
상태코드와 에러 처리 하는 부분에서 각각 api 마다 상태코드에서 에러코드의 케이스가 다르더라구요...!
예를 들어 상태코드가 400이고 에러 코드가 01이라면 회원가입에서는 "username 혹은 password 혹은 hobby가 8자를 넘기는 경우"이고, 유저로그인에서는 "request body가 유효하지 않은 경우"더라고요...!
(참고로 회원가입에서는 400에 00이 reqeust body가 유효하지 못한 경우입니다!)
그래서 이런 부분들을 처리하느라 조금 어려웠습니다... 같으면 좋았읉텐데 하하
제가 서버를 잘 몰라서 그런걸 수도 있는데 아예 같은걸로 통일을 하면 안되는건가요...? 다들 아시는게 있다면 말해주시와요,...!
이번주 코드리뷰도 잘 부탁드립니다!